Skip to content

fix(api-server,control-plane): replace remaining AutoMigrate with raw SQL, fix CP_RUNTIME_NAMESPACE#1442

Merged
mergify[bot] merged 3 commits intomainfrom
fix/migration-automigrate-parameter-mismatch-v2
Apr 23, 2026
Merged

fix(api-server,control-plane): replace remaining AutoMigrate with raw SQL, fix CP_RUNTIME_NAMESPACE#1442
mergify[bot] merged 3 commits intomainfrom
fix/migration-automigrate-parameter-mismatch-v2

Conversation

@markturansky
Copy link
Copy Markdown
Contributor

@markturansky markturansky commented Apr 23, 2026

Summary

  • Replace AutoMigrate in agents, inbox, and credentials initial migrations with raw CREATE TABLE IF NOT EXISTS + CREATE INDEX IF NOT EXISTS — fixes pq: got 2 parameters but the statement requires 1 on non-fresh databases where tables already exist
  • Fix CP_RUNTIME_NAMESPACE defaulting to MPP-specific ambient-code--runtime-int on vanilla OpenShift by falling back to NAMESPACE env var and adding downward API to base manifest

Root Cause

GORM's postgres migrator injects pgx.QueryExecModeSimpleProtocol as a query parameter in GetRows() when DriverName is empty. Since rh-trex-ai uses lib/pq (not native pgx), this sentinel is counted as an extra bind parameter, causing the mismatch. This only triggers when AutoMigrate finds an already-existing table (ALTER TABLE path calls ColumnTypesGetRows).

Test plan

  • go vet ./... and go build ./... pass for both api-server and control-plane
  • Full api-server test suite passes (migrations run on fresh testcontainer PostgreSQL)
  • CI builds image
  • Deploy to UAT/Stage and verify migration init container succeeds

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Updated the ambient control plane to dynamically derive its runtime namespace from the pod's actual namespace instead of using a fixed default value. This improves namespace configuration flexibility and ensures the control plane correctly identifies its runtime environment.

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 23, 2026

Deploy Preview for cheerful-kitten-f556a0 canceled.

Name Link
🔨 Latest commit 633b3c4
🔍 Latest deploy log https://app.netlify.com/projects/cheerful-kitten-f556a0/deploys/69ea7912694c66000910ada1

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4e25fe34-2468-4be1-9538-e7da90b27e7c

📥 Commits

Reviewing files that changed from the base of the PR and between 87c94ea and 3774e0c.

📒 Files selected for processing (2)
  • components/ambient-control-plane/internal/config/config.go
  • components/manifests/base/ambient-control-plane-service.yml
🚧 Files skipped from review as they are similar to previous changes (2)
  • components/manifests/base/ambient-control-plane-service.yml
  • components/ambient-control-plane/internal/config/config.go

📝 Walkthrough

Walkthrough

The CPRuntimeNamespace configuration now dynamically derives from the pod's namespace via the CP_RUNTIME_NAMESPACE environment variable, replacing a static fallback value. The manifest update injects this variable from pod metadata.

Changes

Cohort / File(s) Summary
Dynamic Runtime Namespace Configuration
components/ambient-control-plane/internal/config/config.go, components/manifests/base/ambient-control-plane-service.yml
Replaces fixed CPRuntimeNamespace fallback with dynamic derivation from pod's namespace. Manifest injects CP_RUNTIME_NAMESPACE env var from metadata.namespace; config.go uses it as fallback instead of hardcoded "ambient-code--runtime-int".
🚥 Pre-merge checks | ✅ 5 | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive Title follows Conventional Commits format with type 'fix', scope list, and clear description. However, the raw_summary shows only CP_RUNTIME_NAMESPACE changes, not the AutoMigrate replacements mentioned in the title. Verify that the changeset includes AutoMigrate-to-SQL replacements in agents, inbox, and credentials migrations. If only CP_RUNTIME_NAMESPACE changes are present, update title to reflect actual changes.
Kubernetes Resource Safety ❓ Inconclusive Manifest file referenced in PR could not be accessed; unable to assess OwnerReferences, resource limits, RBAC, namespace scoping, or pod security contexts. Provide access to components/manifests/base/ambient-control-plane-service.yml and related RBAC/Deployment resources for complete resource safety evaluation.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Performance And Algorithmic Complexity ✅ Passed PR introduces minimal changes with no O(n²) algorithms, N+1 queries, expensive loops, or unbounded resource growth; environment variable resolution is trivial startup operation and Kubernetes downward API is zero-cost declarative pattern.
Security And Secret Handling ✅ Passed PR modifies database migrations and environment configuration using safe, idempotent DDL patterns with no SQL injection, string interpolation, hardcoded secrets, or sensitive data leaks detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/migration-automigrate-parameter-mismatch-v2
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/migration-automigrate-parameter-mismatch-v2

Comment @coderabbitai help to get the list of available commands and usage tips.

…ace on vanilla OpenShift

CP_RUNTIME_NAMESPACE defaulted to "ambient-code--runtime-int" (MPP-specific)
in Go code, causing crashloop on vanilla OpenShift where that namespace
doesn't exist. Fix by falling back to NAMESPACE env var (or "ambient-code")
and adding downward API to the base manifest.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@markturansky markturansky force-pushed the fix/migration-automigrate-parameter-mismatch-v2 branch from 87c94ea to d1280c9 Compare April 23, 2026 19:49
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/ambient-api-server/plugins/agents/migration.go`:
- Around line 14-25: The agents table DDL in migration.go creates project_id and
name as nullable, which conflicts with the model's gorm:"not null"; update the
CREATE TABLE statement for agents to add NOT NULL to the project_id and name
column definitions (the SQL string used to create agents) so the schema enforces
the same constraints as the model and prevents schema drift.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5a0ccd58-ed74-4ef8-b08d-804bf0ece811

📥 Commits

Reviewing files that changed from the base of the PR and between d7533c8 and 87c94ea.

📒 Files selected for processing (5)
  • components/ambient-api-server/plugins/agents/migration.go
  • components/ambient-api-server/plugins/credentials/migration.go
  • components/ambient-api-server/plugins/inbox/migration.go
  • components/ambient-control-plane/internal/config/config.go
  • components/manifests/base/ambient-control-plane-service.yml

Comment on lines +14 to +25
`CREATE TABLE IF NOT EXISTS agents (
id TEXT PRIMARY KEY,
created_at TIMESTAMPTZ,
updated_at TIMESTAMPTZ,
deleted_at TIMESTAMPTZ,
project_id TEXT,
name TEXT,
prompt TEXT,
current_session_id TEXT,
labels TEXT,
annotations TEXT
)`,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Restore required constraints in agents DDL

project_id (Line 19) and name (Line 20) are created nullable, but the runtime model treats them as required (gorm:"not null" in components/ambient-api-server/plugins/agents/model.go). This introduces schema drift for fresh installs and weakens data integrity for project-scoped queries.

Proposed fix
 `CREATE TABLE IF NOT EXISTS agents (
 	id TEXT PRIMARY KEY,
 	created_at TIMESTAMPTZ,
 	updated_at TIMESTAMPTZ,
 	deleted_at TIMESTAMPTZ,
-	project_id TEXT,
-	name TEXT,
+	project_id TEXT NOT NULL,
+	name TEXT NOT NULL,
 	prompt TEXT,
 	current_session_id TEXT,
 	labels TEXT,
 	annotations TEXT
 )`,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
`CREATE TABLE IF NOT EXISTS agents (
id TEXT PRIMARY KEY,
created_at TIMESTAMPTZ,
updated_at TIMESTAMPTZ,
deleted_at TIMESTAMPTZ,
project_id TEXT,
name TEXT,
prompt TEXT,
current_session_id TEXT,
labels TEXT,
annotations TEXT
)`,
`CREATE TABLE IF NOT EXISTS agents (
id TEXT PRIMARY KEY,
created_at TIMESTAMPTZ,
updated_at TIMESTAMPTZ,
deleted_at TIMESTAMPTZ,
project_id TEXT NOT NULL,
name TEXT NOT NULL,
prompt TEXT,
current_session_id TEXT,
labels TEXT,
annotations TEXT
)`,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/ambient-api-server/plugins/agents/migration.go` around lines 14 -
25, The agents table DDL in migration.go creates project_id and name as
nullable, which conflicts with the model's gorm:"not null"; update the CREATE
TABLE statement for agents to add NOT NULL to the project_id and name column
definitions (the SQL string used to create agents) so the schema enforces the
same constraints as the model and prevents schema drift.

@mergify mergify Bot added the queued label Apr 23, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Apr 23, 2026

Merge Queue Status

  • Entered queue2026-04-23 20:05 UTC · Rule: default
  • Checks skipped · PR is already up-to-date
  • Merged2026-04-23 20:05 UTC · at 633b3c4895e9c6b27ecf9b7c329d62ed5a7814be · squash

This pull request spent 12 seconds in the queue, including 2 seconds running CI.

Required conditions to merge

@mergify mergify Bot merged commit 9e787d2 into main Apr 23, 2026
55 checks passed
@mergify mergify Bot deleted the fix/migration-automigrate-parameter-mismatch-v2 branch April 23, 2026 20:05
@mergify mergify Bot removed the queued label Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant